-
Notifications
You must be signed in to change notification settings - Fork 441
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
internal: check for new majors in v2 #3078
base: v2-dev
Are you sure you want to change the base?
Conversation
Datadog ReportBranch report: ✅ 0 Failed, 4088 Passed, 64 Skipped, 2m 39.44s Total Time |
BenchmarksBenchmark execution time: 2025-01-24 18:51:49 Comparing candidate commit 46f943c in PR branch Found 0 performance improvements and 0 performance regressions! Performance is the same for 51 metrics, 2 unstable metrics. |
return "", fmt.Errorf("no valid versions found") | ||
} | ||
|
||
// // walk through contrib directory |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TODO(quinna): remove these before merge.
leaving comments for clarity for reviewers.
instrumentation/packages.go
Outdated
@@ -804,6 +804,10 @@ var packages = map[Package]PackageInfo{ | |||
}, | |||
} | |||
|
|||
func GetPackages() map[Package]PackageInfo { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a duplicate from https://github.com/DataDog/dd-trace-go/pull/3070/files#diff-361675d30208c976289abde824bc8391916548f27a769c8264f036a4daf420f1R850
so it depends on which PR gets merged first.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TODO: remove or change this, depending on which PR gets merged first
var stdLibPackages = map[string]struct{}{ | ||
"log/slog": {}, | ||
"os": {}, | ||
"net/http": {}, | ||
"database/sql": {}, | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is redeclared from https://github.com/DataDog/dd-trace-go/pull/3070/files#diff-1196dd5bce1d4f0ed73101590ba38f17605ce38d6c18081f4bdaa2a397ab5adeR72-R77
wonder if we can just put this in instrumentation/packages.go
and export it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Question: would the standard in Go here be:
- declare this as a public variable such as
StandardPackages
ininstrumentation/packages.go
? - create a public method such as
GetStandardPackage
ininstrumentation/packages.go
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Made this StandardPackages
in packages.go; but alternatively, could create a new field in PackageInfo
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! Overall looks good to me, just a few comments
|
||
// Fetch `go.mod` with command | ||
// curl https://raw.githubusercontent.com/<module>/refs/tags/<latest>/go.mod | ||
goModURL := fmt.Sprintf("https://raw.githubusercontent.com/%s/refs/tags/%s/go.mod", base, latest) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was just testing this out with other files
It looks like https://raw.githubusercontent.com/OWNER/REPO/TAG/FILE
also works
Just wondering if there is any difference, not really seeing anything online though for it
Datadog ReportBranch report: ✅ 0 Failed, 4095 Passed, 64 Skipped, 2m 43.83s Total Time |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note: not sure why these packages are showing as changes after rebasing off v2-dev
What does this PR do?
Checks for new major versions on github in v2
Motivation
In the original PR, we used a hardcoded, manual
integration_go.mod
to detect if new major versions (corresponding to new packages) have been released. However, inv2
each package will be treated as its own contrib with its own correspondinggo.mod
. We want to parse thesego.mod
files to check the currentlatest
major, and output if a new major is available.Check the latest major we support for a given package / module (parsing the
go.mod
files).Fetch / check for new major versions from github:
go list -m -json <repository>@<version>
and extractOrigin[url]
git ls-remote --tags <repository>
curl https://raw.githubusercontent.com/<module>/refs/tags/<latest_version>/go.mod
404
, the module is not a go module and we can skip it"New latest major on Github: {github_latest_major}"
Example output from workflow run: https://github.com/DataDog/dd-trace-go/actions/runs/12952678774/job/36130554853
TODO:
instrumentation/packages.go
Reviewer's Checklist
v2-dev
branch and reviewed by @DataDog/apm-go.Unsure? Have a question? Request a review!